Skip to content

fix: correct Minute/Second toString for negative values - #105

Merged
sshanks-kx merged 1 commit into
KxSystems:masterfrom
belowzeroff:fix/minute-second-negative-tostring
Jul 30, 2026
Merged

fix: correct Minute/Second toString for negative values#105
sshanks-kx merged 1 commit into
KxSystems:masterfrom
belowzeroff:fix/minute-second-negative-tostring

Conversation

@belowzeroff

Copy link
Copy Markdown
Contributor

Summary

Minute.toString() and Second.toString() render a stray minus sign on every component for negative values.

kdb+ minute and second values can be negative (e.g. the result of time arithmetic such as neg 01:30). The value was formatted as i2(i/60) : i2(i%60), and because Java integer division and remainder are both negative for a negative operand, each component received its own minus sign:

value before expected
Minute(-90) -01:-30 -01:30
Minute(-30) 00:-30 -00:30
Second(-3661) -01:-01:-01 -01:01:01

Change

Apply the sign once and format the magnitude, mirroring how Timespan.toString() already handles negatives. Positive values — including values beyond 24h, e.g. Minute(1500)25:00 — are unchanged.

Testing

Extended testMinuteToString / testSecondToString with negative and beyond-24h cases. Full suite green (84 tests, no regressions).

kdb+ minute and second values can be negative (e.g. the result of time
arithmetic such as `neg 01:30`). Their toString rendered a minus sign on every
component because Java integer division and remainder are both negative for a
negative operand:

  Minute(-90)   -> "-01:-30"     (expected "-01:30")
  Second(-3661) -> "-01:-01:-01" (expected "-01:01:01")

Apply the sign once and format the magnitude, mirroring how Timespan.toString
already handles negatives. Extend the unit tests with negative and beyond-24h
cases.
@sshanks-kx
sshanks-kx merged commit c3cc419 into KxSystems:master Jul 30, 2026
1 check passed
@belowzeroff
belowzeroff deleted the fix/minute-second-negative-tostring branch July 31, 2026 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants